Skip to content

test(worker-bus): export v2 duckdb/inference handlers + parity gate tests - #286

Merged
qnbs merged 2 commits into
mainfrom
feat/worker-v2-parity-gate
Jul 29, 2026
Merged

test(worker-bus): export v2 duckdb/inference handlers + parity gate tests#286
qnbs merged 2 commits into
mainfrom
feat/worker-v2-parity-gate

Conversation

@qnbs

@qnbs qnbs commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

First of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication tracked in ADR-0014. Plan: .claude/plans/worker-generation-v1-vs-enumerated-fox.md.

  • Export initDuckDb/handleQuery/handleExec/handleShutdown (workers/v2/duckdb.worker.ts) and handleInference (workers/v2/inference.worker.ts), matching how webllm.worker.ts's handleWebLlm is already exported and unit-tested.
  • Add tests/unit/duckdbWorkerHandler.test.ts (10 tests) and tests/unit/inferenceWorkerHandlerV2.test.ts (10 tests), mirroring tests/unit/webllmWorkerHandler.test.ts's pattern β€” mock the external library, exercise the real handler logic directly, no live worker/WorkerBus needed.
  • Real bug found and fixed: initDuckDb()'s OPFS-unavailable catch block silently swallowed the failure. v1 (workers/duckdbWorker.ts) posts an out-of-band OPFS_FALLBACK message so the UI can warn the user their analytics won't persist across reloads (private browsing, old Safari); v2 had the identical catch block but emitted nothing. Fixed by threading ctx.emitProgress into initDuckDb() and emitting an 'opfs-fallback' progress stage β€” reuses the existing progress channel (same one WebLLM uses for download progress), no new message-protocol/schema change.

Both v2 worker files had zero production callers before this PR (confirmed via repo-wide grep) β€” they were registered as live WorkerBus v2 pools but never exercised by real traffic, so this parity gate exists specifically to catch drift like the OPFS gap before any consumer (duckdbClient.ts, localEmbeddingService.ts, localNlpService.ts) cuts over in the next 3 PRs.

Test plan

  • pnpm exec vitest run tests/unit/duckdbWorkerHandler.test.ts tests/unit/inferenceWorkerHandlerV2.test.ts tests/unit/webllmWorkerHandler.test.ts β€” 25/25 passing
  • pnpm run typecheck (exact CI command) β€” clean
  • pnpm run lint (Biome, staged files) β€” clean via pre-commit hook
  • CI quality gate green (full coverage run β€” not run locally per this repo's low-end-hardware CI-cloud-first policy)

πŸ€– Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • DuckDB now emits progress details when OPFS setup fails and cleanly falls back to a supported connection path.
    • DuckDB query, execution, and shutdown behaviors are now more consistent, including initialization error handling.
    • Inference results are normalized consistently across embedding/sentiment/summary/text-generation, with proper cancellation handling.
  • Tests

    • Added unit tests for the DuckDB worker v2 lifecycle, OPFS fallback scenarios, query/exec, and shutdown.
    • Added unit tests for inference handling across supported task types, output normalization, cancellation, and inference option forwarding.

…ests

Neither workers/v2/duckdb.worker.ts's handlers nor
workers/v2/inference.worker.ts's handleInference were exported or
unit-tested against real logic (only webllm.worker.ts had that,
tests/unit/webllmWorkerHandler.test.ts) despite both already being
registered as live WorkerBus v2 pools with zero production callers β€”
docs/adr/0014-worker-generation-duplication.md's deferred migration
needs these to actually match v1 before any consumer cuts over.

Testing surfaced a real gap: initDuckDb()'s OPFS-unavailable catch
block silently swallowed the failure, unlike v1's out-of-band
OPFS_FALLBACK message that lets the UI warn users their analytics
won't persist. Fixed by threading ctx.emitProgress into initDuckDb()
and emitting an 'opfs-fallback' progress stage from the catch block β€”
reuses the existing progress channel WebLLM already uses for download
progress, no new message-protocol/schema change needed.

First PR of the ADR-0014 consolidation sprint (5 PRs total, see
.claude/plans/worker-generation-v1-vs-enumerated-fox.md).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
worldscript-studio Ready Ready Preview, Comment Jul 29, 2026 5:46pm

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

βš™οΈ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c003d8fd-470b-41f6-988b-786d99d5197c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • πŸ” Trigger review
πŸ“ Walkthrough

Walkthrough

Adds direct unit-test coverage for v2 DuckDB and inference worker handlers. DuckDB initialization now reports OPFS fallback progress, and both workers export handlers for test access.

Changes

DuckDB worker handlers

Layer / File(s) Summary
Handler exports and OPFS fallback reporting
workers/v2/duckdb.worker.ts
Exports DuckDB handlers, accepts an optional progress emitter during initialization, cleans up partial OPFS connections, and emits opfs-fallback when OPFS setup fails.
Handler behavior tests
tests/unit/duckdbWorkerHandler.test.ts
Tests mocked initialization, OPFS behavior, query and exec responses, initialization errors, and shutdown cleanup.

Inference worker handler

Layer / File(s) Summary
Handler export
workers/v2/inference.worker.ts
Exports handleInference without changing its runtime logic or task wiring.
Inference behavior tests
tests/unit/inferenceWorkerHandlerV2.test.ts
Tests abort handling, embedding normalization, progress events, sentiment formatting, text extraction, and option forwarding.

Estimated code review effort: 3 (Moderate) | ~20 minutes

πŸš₯ Pre-merge checks | βœ… 5
βœ… Passed checks (5 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The title clearly matches the PR’s main change: exporting v2 DuckDB/inference handlers and adding parity-focused tests.
Docstring Coverage βœ… Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check βœ… Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check βœ… Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
πŸ“ Generate docstrings
  • Create stacked PR
  • Commit on current branch
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/worker-v2-parity-gate

Comment @coderabbitai help to get the list of available commands.

@codeant-ai

codeant-ai Bot commented Jul 29, 2026

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: bcc41c27
Scan Time: 2026-07-29 17:46:34 UTC

βœ… Overall Status: PASSED

Quality Gate Details

Quality Gate Status Details
Secrets βœ… PASSED 0 secrets found
Duplicate Code βœ… PASSED 1.8% duplicated
SAST βœ… PASSED No security issues
Bugs βœ… PASSED Rating S: No bugs
IAC βœ… PASSED No IAC issues

View Full Results

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

πŸ€– Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@workers/v2/duckdb.worker.ts`:
- Around line 45-46: Replace the multi-line QNBS-v3 annotations with one
immediate comment in the required β€œ// QNBS-v3: [Reason / impact / creative
value]” format: workers/v2/duckdb.worker.ts lines 45-46 for the export rationale
and lines 76-79 for the fallback-channel rationale;
tests/unit/duckdbWorkerHandler.test.ts lines 2-5 for the suite rationale, 17-18
for hoisting, 31-32 for constructability, 58 for import order, 91-92 for the
shutdown mock, and 99-101 for state reset. Preserve each annotation’s original
rationale while collapsing it to a single bracketed line.
- Around line 75-85: Update workers/v2/duckdb.worker.ts lines 75-85 to retain
the OPFS connection separately, close it in the ATTACH failure catch path, then
create and assign the fallback connection. Update
tests/unit/duckdbWorkerHandler.test.ts lines 150-157 to use distinct OPFS and
fallback connections and assert the OPFS connection’s close() method is called.

In `@workers/v2/inference.worker.ts`:
- Around line 45-46: Replace the multi-line QNBS annotations with adjacent
single-line bracketed comments using the required β€œQNBS-v3: [Grund / Impact /
Kreativer Mehrwert]” format: update the handler export annotation in
workers/v2/inference.worker.ts at lines 45-46, collapse the file-purpose
annotation in tests/unit/inferenceWorkerHandlerV2.test.ts at lines 2-6, and
reformat the import-order annotation at line 17. Preserve each annotation’s
original intent.
πŸͺ„ Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
βš™οΈ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4df0d1fa-34af-4230-be78-c6bafca5f8de

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between dc14bc0 and b4b1bbf.

πŸ“’ Files selected for processing (4)
  • tests/unit/duckdbWorkerHandler.test.ts
  • tests/unit/inferenceWorkerHandlerV2.test.ts
  • workers/v2/duckdb.worker.ts
  • workers/v2/inference.worker.ts

Comment thread workers/v2/duckdb.worker.ts Outdated
Comment thread workers/v2/duckdb.worker.ts
Comment thread workers/v2/inference.worker.ts Outdated
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.

πŸ“’ Thoughts on this report? Let us know!

…S-v3 format

CodeRabbit review on #286 found a real bug: initDuckDb()'s OPFS catch
block overwrote `connection` with the fallback connection without
closing the first (partially-attached) one, leaking a DuckDB
connection on every OPFS-attach failure. Fixed by tracking the OPFS
attempt in its own local variable and closing it in the catch path
before falling back. New regression test asserts close() is called.

Also collapsed several multi-line QNBS-v3 comments in
workers/v2/duckdb.worker.ts, workers/v2/inference.worker.ts, and both
new test files to the required single bracketed-line format.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@qnbs

qnbs commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
βœ… Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
workers/v2/duckdb.worker.ts (1)

81-87: 🩺 Stability & Availability | 🟠 Major | ⚑ Quick win

Keep OPFS fallback running if cleanup fails. await opfsConnection?.close() can reject inside this catch, which skips emitProgress('opfs-fallback', ...) and the fallback newDb.connect(), and also hides the original OPFS error. Make cleanup best-effort, log the close failure separately, and still continue to the fallback connection. Add a regression test where close() rejects but fallback init succeeds.

πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@workers/v2/duckdb.worker.ts` around lines 81 - 87, Make OPFS cleanup in the
fallback catch best-effort: wrap opfsConnection.close() in its own error
handling, log any close failure separately without replacing the original OPFS
error, then always emit the opfs-fallback progress event and run
newDb.connect(). Add a regression test covering a rejecting close() with
successful fallback initialization.

Source: Coding guidelines

πŸ€– Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@workers/v2/duckdb.worker.ts`:
- Around line 81-87: Make OPFS cleanup in the fallback catch best-effort: wrap
opfsConnection.close() in its own error handling, log any close failure
separately without replacing the original OPFS error, then always emit the
opfs-fallback progress event and run newDb.connect(). Add a regression test
covering a rejecting close() with successful fallback initialization.

ℹ️ Review info
βš™οΈ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 061cd04e-91f6-4502-9d44-911a9ca35e0a

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between b4b1bbf and bcc41c2.

πŸ“’ Files selected for processing (4)
  • tests/unit/duckdbWorkerHandler.test.ts
  • tests/unit/inferenceWorkerHandlerV2.test.ts
  • workers/v2/duckdb.worker.ts
  • workers/v2/inference.worker.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • workers/v2/inference.worker.ts
  • tests/unit/duckdbWorkerHandler.test.ts
  • tests/unit/inferenceWorkerHandlerV2.test.ts

@qnbs
qnbs merged commit ea879e4 into main Jul 29, 2026
26 checks passed
@qnbs
qnbs deleted the feat/worker-v2-parity-gate branch July 29, 2026 18:10
@qnbs

qnbs commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Addressed the outside-diff-range finding from the second CodeRabbit pass (workers/v2/duckdb.worker.ts:81-87, "Keep OPFS fallback running if cleanup fails") in dbebf5c: opfsConnection?.close() is now wrapped in its own try/catch β€” a rejecting close() no longer skips the opfs-fallback progress emit or the fallback connect(), and no longer replaces the original ATTACH error. Added a regression test covering a rejecting close() with successful fallback init.

@qnbs

qnbs commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
βœ… Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

qnbs added a commit that referenced this pull request Jul 30, 2026
#287)

* test(worker-bus): export v2 duckdb/inference handlers + parity gate tests

Neither workers/v2/duckdb.worker.ts's handlers nor
workers/v2/inference.worker.ts's handleInference were exported or
unit-tested against real logic (only webllm.worker.ts had that,
tests/unit/webllmWorkerHandler.test.ts) despite both already being
registered as live WorkerBus v2 pools with zero production callers β€”
docs/adr/0014-worker-generation-duplication.md's deferred migration
needs these to actually match v1 before any consumer cuts over.

Testing surfaced a real gap: initDuckDb()'s OPFS-unavailable catch
block silently swallowed the failure, unlike v1's out-of-band
OPFS_FALLBACK message that lets the UI warn users their analytics
won't persist. Fixed by threading ctx.emitProgress into initDuckDb()
and emitting an 'opfs-fallback' progress stage from the catch block β€”
reuses the existing progress channel WebLLM already uses for download
progress, no new message-protocol/schema change needed.

First PR of the ADR-0014 consolidation sprint (5 PRs total, see
.claude/plans/worker-generation-v1-vs-enumerated-fox.md).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): close leaked OPFS connection on ATTACH failure + QNBS-v3 format

CodeRabbit review on #286 found a real bug: initDuckDb()'s OPFS catch
block overwrote `connection` with the fallback connection without
closing the first (partially-attached) one, leaking a DuckDB
connection on every OPFS-attach failure. Fixed by tracking the OPFS
attempt in its own local variable and closing it in the catch path
before falling back. New regression test asserts close() is called.

Also collapsed several multi-line QNBS-v3 comments in
workers/v2/duckdb.worker.ts, workers/v2/inference.worker.ts, and both
new test files to the required single bracketed-line format.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(duckdb): migrate duckdbClient onto WorkerBus v2, delete v1 worker

Second of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication
(ADR-0014). services/duckdb/duckdbClient.ts's public API (init/query/
exec/shutdown/terminate/setOpfsFallbackHandler) is unchanged β€” its 5
real consumers (useDuckDb.ts, duckdbAnalytics.ts, duckdbMigration.ts,
ragVectorMigration.ts, telemetryService.ts) need no changes. Only the
internals swapped from a raw `new Worker(duckdbWorker.ts)` to routing
through the shared WorkerBus v2 'duckdb' pool via a new
ensureDuckDbPool() (mirrors ensureWebLlmPool()'s decoupling from the
enableWorkerBusV2 flag β€” analytics is core, not experimental infra).

The v2 worker returns raw values and throws on failure, unlike v1's
{ok,rows,error} wrapper β€” the adapter in duckdbClient.ts's send()
translates at that one boundary instead of touching ~20 call sites
across 5 files. WorkerBus's default 2-retry is disabled per-call
(retryPolicy: {maxRetries:0}) to avoid stacking under
duckdbAnalytics.ts's existing withDuckDbRetry() app-level retry.

Two things surfaced while wiring this up, both fixed here:
- WorkerBus.shutdown() tears down all 4 pools with no way to scope to
  just 'duckdb' β€” added WorkerBus.terminatePool(poolId) so
  duckdbClient.terminate() doesn't kill in-flight inference/webllm/
  plugin work on the shared bus.
- send() is async and awaits ensureDuckDbPool() before a task handle
  exists; naively attaching the AbortSignal listener after that await
  (like a first draft did) can lose a same-tick abort, unlike v1's
  synchronous Promise-executor version. Fixed by attaching the
  listener synchronously up front and applying it once the handle is
  ready β€” regression-tested.

workers/duckdbWorker.ts deleted; features/featureCatalog.ts's
enableDuckDbAnalytics entry retargeted to workers/v2/duckdb.worker.ts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): make OPFS-connection cleanup best-effort on ATTACH failure

CodeRabbit's second pass on #286 flagged this as an outside-diff-range
comment (not a resolvable inline thread) on bcc41c2's own fix: the
new `await opfsConnection?.close()` call could itself reject, which
would skip emitProgress('opfs-fallback', ...) and the fallback
newDb.connect(), and replace the original ATTACH error with the close
failure β€” turning a graceful degrade into a hard init failure.

Wrapped the cleanup in its own try/catch (best-effort, logged via
console.warn, never blocks the fallback path) so a failing close()
can't prevent the fallback connection or swallow the real error. New
regression test asserts both still happen when close() rejects.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): pool-termination lifecycle, idle-respawn recovery, SQL param binding

Three real findings from CodeRabbit's review of #287 (duckdbClient
migration onto WorkerBus v2), all confirmed against current code
before fixing:

1. CRITICAL β€” params silently dropped (workers/v2/duckdb.worker.ts).
   duckdbClient.send() enqueues {sql, params}, but handleQuery/
   handleExec only read req.sql and called connection.query(sql)
   directly, ignoring params entirely and leaving an unbound-query/
   SQL-injection surface flagged by static analysis. This was already
   live: services/ai/telemetryService.ts's writeToDuckDb() passes real
   bound params ([taskType, backend, modelId, latencyMs, success])
   that were being silently discarded. Fixed via DuckDB-WASM prepared
   statements (connection.prepare(sql) + stmt.query(...params) +
   stmt.close()) when params is non-empty; unchanged direct
   connection.query(sql) otherwise. Needed a narrow structural type
   (PreparableConnection) since tsgo doesn't resolve
   AsyncDuckDBConnection.prepare() through this package's nested
   `export *` chain, despite it being present in the package's own
   .d.ts β€” same class of tsgo/external-package gap CLAUDE.md already
   documents for the transformers.js path alias.

2. MAJOR β€” DuckDB pool worker losing its connection on respawn
   (services/duckdb/duckdbClient.ts). The duckdb pool's minWorkers:0
   lets its worker idle-terminate; a freshly spawned replacement
   worker's module state has no `connection` until INIT runs again on
   it, so the next QUERY/EXEC after a respawn failed with "DuckDB not
   initialized". Fixed by transparently re-sending INIT once and
   retrying the original call when send() sees that specific error
   message (bounded to one retry, INIT/SHUTDOWN excluded from the
   check to avoid any recursive loop).

3. MAJOR β€” terminatePool() had no lifecycle contract
   (packages/worker-bus/src/workerBus.ts +
   services/workerBusManager.ts). Removing a pool didn't settle tasks
   already routed to it β€” their result promises would await a RESULT
   message from a worker that no longer exists. Fixed by tracking
   task->pool assignment (new taskPools map, set/cleared around
   runTask's pool-acquire phase) and cancelling matching tasks before
   deletion. Separately, ensureDuckDbPool() assumed a non-null bus
   always has every pool, so any call after duckdbClient.terminate()
   would fail with NO_POOL forever; added WorkerBus.hasPool() and a
   re-registration path (duckDbPoolOptions()/reRegisterDuckDbPool(),
   shared with doInitWorkerBus() to avoid option drift between the two
   registration sites).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: trigger CI after auto-retarget to main

* fix(worker): route OPFS-close failure through services/logger.ts

Matches the log.warn/log.error convention services/workerBusManager.ts
already uses in this PR β€” services/logger.ts is worker-safe (guarded
window access, lazy Tauri import), so raw console.warn was an
unnecessary inconsistency, not a technical constraint.

Addresses CodeRabbit nitpick on PR #287.

* test(worker-bus,duckdb): close Codecov patch-coverage gaps on PR #287

- duckdbClient.ts: cover the String(err) fallback branch in
  errorMessage(), the opfs-fallback message ?? default, and the
  reinit.ok === false path (INIT retry itself failing).
- workerBus.ts: add direct hasPool() coverage (true/false/after
  terminatePool) and a throwing-subscriber test that reaches
  emit()'s isolation catch via the circuit-breaker-open event path
  (the only two call sites of emit() in the class).

Addresses the Codecov "patch coverage" report on PR #287 (4 lines
missing coverage across these two files).

* fix(duckdb): remove abort listener after send() settles

A caller that reuses one AbortSignal across many queries (a common
hook/thunk pattern) would otherwise accumulate one 'abort' listener per
call for the signal's whole lifetime, since {once:true} only self-removes
when the signal actually fires.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* test(worker-bus): use resetAllMocks to prevent mock bleed in inferenceWorkerHandlerV2 tests

clearAllMocks clears call history but not mockImplementation/mockResolvedValue,
so a test could inherit mockPipelineFactory's return value from an earlier
test instead of setting its own explicitly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(test): address CodeRabbit review nitpicks on PR #287

- duckdbClient.test.ts: move OPFS fallback-handler reset into a suite-level
  afterEach so it still runs when an assertion throws mid-test, instead of
  three end-of-test reset calls that could be skipped.
- inferenceWorkerHandlerV2.test.ts: collapse the QNBS-v3 rationale comment
  to the required single-line format.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
qnbs added a commit that referenced this pull request Jul 30, 2026
* test(worker-bus): export v2 duckdb/inference handlers + parity gate tests

Neither workers/v2/duckdb.worker.ts's handlers nor
workers/v2/inference.worker.ts's handleInference were exported or
unit-tested against real logic (only webllm.worker.ts had that,
tests/unit/webllmWorkerHandler.test.ts) despite both already being
registered as live WorkerBus v2 pools with zero production callers β€”
docs/adr/0014-worker-generation-duplication.md's deferred migration
needs these to actually match v1 before any consumer cuts over.

Testing surfaced a real gap: initDuckDb()'s OPFS-unavailable catch
block silently swallowed the failure, unlike v1's out-of-band
OPFS_FALLBACK message that lets the UI warn users their analytics
won't persist. Fixed by threading ctx.emitProgress into initDuckDb()
and emitting an 'opfs-fallback' progress stage from the catch block β€”
reuses the existing progress channel WebLLM already uses for download
progress, no new message-protocol/schema change needed.

First PR of the ADR-0014 consolidation sprint (5 PRs total, see
.claude/plans/worker-generation-v1-vs-enumerated-fox.md).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): close leaked OPFS connection on ATTACH failure + QNBS-v3 format

CodeRabbit review on #286 found a real bug: initDuckDb()'s OPFS catch
block overwrote `connection` with the fallback connection without
closing the first (partially-attached) one, leaking a DuckDB
connection on every OPFS-attach failure. Fixed by tracking the OPFS
attempt in its own local variable and closing it in the catch path
before falling back. New regression test asserts close() is called.

Also collapsed several multi-line QNBS-v3 comments in
workers/v2/duckdb.worker.ts, workers/v2/inference.worker.ts, and both
new test files to the required single bracketed-line format.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(duckdb): migrate duckdbClient onto WorkerBus v2, delete v1 worker

Second of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication
(ADR-0014). services/duckdb/duckdbClient.ts's public API (init/query/
exec/shutdown/terminate/setOpfsFallbackHandler) is unchanged β€” its 5
real consumers (useDuckDb.ts, duckdbAnalytics.ts, duckdbMigration.ts,
ragVectorMigration.ts, telemetryService.ts) need no changes. Only the
internals swapped from a raw `new Worker(duckdbWorker.ts)` to routing
through the shared WorkerBus v2 'duckdb' pool via a new
ensureDuckDbPool() (mirrors ensureWebLlmPool()'s decoupling from the
enableWorkerBusV2 flag β€” analytics is core, not experimental infra).

The v2 worker returns raw values and throws on failure, unlike v1's
{ok,rows,error} wrapper β€” the adapter in duckdbClient.ts's send()
translates at that one boundary instead of touching ~20 call sites
across 5 files. WorkerBus's default 2-retry is disabled per-call
(retryPolicy: {maxRetries:0}) to avoid stacking under
duckdbAnalytics.ts's existing withDuckDbRetry() app-level retry.

Two things surfaced while wiring this up, both fixed here:
- WorkerBus.shutdown() tears down all 4 pools with no way to scope to
  just 'duckdb' β€” added WorkerBus.terminatePool(poolId) so
  duckdbClient.terminate() doesn't kill in-flight inference/webllm/
  plugin work on the shared bus.
- send() is async and awaits ensureDuckDbPool() before a task handle
  exists; naively attaching the AbortSignal listener after that await
  (like a first draft did) can lose a same-tick abort, unlike v1's
  synchronous Promise-executor version. Fixed by attaching the
  listener synchronously up front and applying it once the handle is
  ready β€” regression-tested.

workers/duckdbWorker.ts deleted; features/featureCatalog.ts's
enableDuckDbAnalytics entry retargeted to workers/v2/duckdb.worker.ts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): make OPFS-connection cleanup best-effort on ATTACH failure

CodeRabbit's second pass on #286 flagged this as an outside-diff-range
comment (not a resolvable inline thread) on bcc41c2's own fix: the
new `await opfsConnection?.close()` call could itself reject, which
would skip emitProgress('opfs-fallback', ...) and the fallback
newDb.connect(), and replace the original ATTACH error with the close
failure β€” turning a graceful degrade into a hard init failure.

Wrapped the cleanup in its own try/catch (best-effort, logged via
console.warn, never blocks the fallback path) so a failing close()
can't prevent the fallback connection or swallow the real error. New
regression test asserts both still happen when close() rejects.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(ai): migrate localEmbeddingService onto WorkerBus v2

Third of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication
(ADR-0014). services/ai/localEmbeddingService.ts's public API
(embedText/embedBatch/cosineSimilarity/clearEmbeddingCache) is
unchanged β€” its consumers (localRagService.ts, crossProjectIndexService.ts,
ragPromptAssembly.ts, ragVectorMigration.ts, proForgeMemoryBank.ts,
loraEvaluationService.ts, loraDatasetBuilder.ts, inferenceGateway.ts)
need no changes. Internals swapped from a dedicated
`new Worker(inference.worker.ts)` instance to the shared WorkerBus v2
'inference' pool via ensureInferencePool() (same decoupling-from-
enableWorkerBusV2 pattern as ensureWebLlmPool/ensureDuckDbPool).

v2's handler already throws on failure (matching embedText's existing
throw-on-failure contract), so no response-shape translation is
needed here β€” just swap the transport. Deleted ~70 lines of hand-rolled
health-check/restart/backoff logic (30s ping, exponential backoff up
to 5 attempts), superseded by WorkerPool's generic health check.

Capped the shared 'inference' pool's maxWorkers to 2 (was
MAX_WORKERS_INFERENCE=4) in workerBusManager.ts: each pool replica
independently loads its own transformers.js pipeline with no
cross-replica cache sharing, so 4 concurrent workers under a burst
(e.g. embedBatch's micro-batching) could mean 4x the model memory
footprint.

workers/inference.worker.ts (v1) is NOT deleted yet β€” localNlpService.ts
still uses it directly; removal is PR 4 of this sequence, after that
migration lands too.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): pool-termination lifecycle, idle-respawn recovery, SQL param binding

Three real findings from CodeRabbit's review of #287 (duckdbClient
migration onto WorkerBus v2), all confirmed against current code
before fixing:

1. CRITICAL β€” params silently dropped (workers/v2/duckdb.worker.ts).
   duckdbClient.send() enqueues {sql, params}, but handleQuery/
   handleExec only read req.sql and called connection.query(sql)
   directly, ignoring params entirely and leaving an unbound-query/
   SQL-injection surface flagged by static analysis. This was already
   live: services/ai/telemetryService.ts's writeToDuckDb() passes real
   bound params ([taskType, backend, modelId, latencyMs, success])
   that were being silently discarded. Fixed via DuckDB-WASM prepared
   statements (connection.prepare(sql) + stmt.query(...params) +
   stmt.close()) when params is non-empty; unchanged direct
   connection.query(sql) otherwise. Needed a narrow structural type
   (PreparableConnection) since tsgo doesn't resolve
   AsyncDuckDBConnection.prepare() through this package's nested
   `export *` chain, despite it being present in the package's own
   .d.ts β€” same class of tsgo/external-package gap CLAUDE.md already
   documents for the transformers.js path alias.

2. MAJOR β€” DuckDB pool worker losing its connection on respawn
   (services/duckdb/duckdbClient.ts). The duckdb pool's minWorkers:0
   lets its worker idle-terminate; a freshly spawned replacement
   worker's module state has no `connection` until INIT runs again on
   it, so the next QUERY/EXEC after a respawn failed with "DuckDB not
   initialized". Fixed by transparently re-sending INIT once and
   retrying the original call when send() sees that specific error
   message (bounded to one retry, INIT/SHUTDOWN excluded from the
   check to avoid any recursive loop).

3. MAJOR β€” terminatePool() had no lifecycle contract
   (packages/worker-bus/src/workerBus.ts +
   services/workerBusManager.ts). Removing a pool didn't settle tasks
   already routed to it β€” their result promises would await a RESULT
   message from a worker that no longer exists. Fixed by tracking
   task->pool assignment (new taskPools map, set/cleared around
   runTask's pool-acquire phase) and cancelling matching tasks before
   deletion. Separately, ensureDuckDbPool() assumed a non-null bus
   always has every pool, so any call after duckdbClient.terminate()
   would fail with NO_POOL forever; added WorkerBus.hasPool() and a
   re-registration path (duckDbPoolOptions()/reRegisterDuckDbPool(),
   shared with doInitWorkerBus() to avoid option drift between the two
   registration sites).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(ai): add the missing ensureInferencePool export that broke prod builds

Root cause of both Vercel deployment failures on this branch/PR chain:
services/ai/localEmbeddingService.ts imports and calls
ensureInferencePool() from workerBusManager.ts, but that function was
never actually added there β€” only ensureDuckDbPool() was. Vitest never
caught this because localEmbeddingService.test.ts mocks the entire
workerBusManager module (vi.mock(...) replaces it wholesale), so the
real file's missing export was invisible to that suite. tsgo's
typecheck also passed clean locally for reasons still unclear (worth
a follow-up look), but rolldown's stricter static bundling in the
actual `pnpm run build:edge` (Vercel's exact build command) correctly
failed hard with [MISSING_EXPORT] β€” reproduced locally with
`NODE_OPTIONS=--max-old-space-size=3072 pnpm run build:edge`, which is
what actually surfaced this.

Added ensureInferencePool(), mirroring ensureDuckDbPool()'s shape
(decoupled from enableWorkerBusV2, re-registers the pool if it was
removed via terminatePool() while the bus stays alive). Refactored
doInitWorkerBus()'s inline 'inference' pool registration to share the
same inferencePoolOptions() the new function uses, avoiding the same
kind of drift the duckdb pool options already guard against.

New tests in workerBusManager.test.ts import the *real* module (only
@domain/worker-bus is mocked) β€” this exact suite would have caught the
missing export immediately, unlike the fully-mocked consumer-side
test. Verified fixed via a full local `pnpm run build:edge`
reproduction of Vercel's exact build command (previously failed with
[MISSING_EXPORT], now builds clean).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): guard pool re-registration failures in workerBusManager

ensureDuckDbPool() and ensureInferencePool() could reject after _bus was
already live because their re-registration helpers awaited a lazy import and
called registerPool() without a catch, breaking the documented "null only if
init failed" contract. Wrap both paths so a failed re-add logs instead of
propagating, and add a maxWorkers:2 assertion to the inference re-registration
test so the pool cap regression would be caught.

Addresses CodeRabbit review comments on PR #288.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* test(worker-bus): close Codecov patch-coverage gap on PR #288

The prior CodeRabbit fix (8a63b7c) added try/catch guards in
ensureDuckDbPool()/ensureInferencePool() so a pool re-registration failure
logs instead of propagating, but no test exercised the catch path β€” Codecov
flagged 4 missing lines (83.33% patch coverage). Adds one test per function
that forces registerPool() to throw and asserts the live bus is still
returned and the failure is logged via services/logger.ts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(worker-bus): add QNBS-v3 rationale comments per CodeRabbit outside-diff-range review

Addresses CodeRabbit's 2 still-valid findings on PR #288: the logger mock
(why it's isolated from the real StructuredLogger) and the maxWorkers:2
regression assertion (why 2, not just what). The other 3 flagged spots
(reRegisterInferencePool, the inference registry.register block, and the
mockLogError.mockClear() line) mirror already-uncommented sibling code in
the same files (reRegisterDuckDbPool, the duckdb registry.register block,
6 other .mockClear() calls) β€” commenting only the new instance would be
less consistent, not more, so those are left as-is.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
qnbs added a commit that referenced this pull request Jul 30, 2026
…nce worker (#290)

* test(worker-bus): export v2 duckdb/inference handlers + parity gate tests

Neither workers/v2/duckdb.worker.ts's handlers nor
workers/v2/inference.worker.ts's handleInference were exported or
unit-tested against real logic (only webllm.worker.ts had that,
tests/unit/webllmWorkerHandler.test.ts) despite both already being
registered as live WorkerBus v2 pools with zero production callers β€”
docs/adr/0014-worker-generation-duplication.md's deferred migration
needs these to actually match v1 before any consumer cuts over.

Testing surfaced a real gap: initDuckDb()'s OPFS-unavailable catch
block silently swallowed the failure, unlike v1's out-of-band
OPFS_FALLBACK message that lets the UI warn users their analytics
won't persist. Fixed by threading ctx.emitProgress into initDuckDb()
and emitting an 'opfs-fallback' progress stage from the catch block β€”
reuses the existing progress channel WebLLM already uses for download
progress, no new message-protocol/schema change needed.

First PR of the ADR-0014 consolidation sprint (5 PRs total, see
.claude/plans/worker-generation-v1-vs-enumerated-fox.md).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): close leaked OPFS connection on ATTACH failure + QNBS-v3 format

CodeRabbit review on #286 found a real bug: initDuckDb()'s OPFS catch
block overwrote `connection` with the fallback connection without
closing the first (partially-attached) one, leaking a DuckDB
connection on every OPFS-attach failure. Fixed by tracking the OPFS
attempt in its own local variable and closing it in the catch path
before falling back. New regression test asserts close() is called.

Also collapsed several multi-line QNBS-v3 comments in
workers/v2/duckdb.worker.ts, workers/v2/inference.worker.ts, and both
new test files to the required single bracketed-line format.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(duckdb): migrate duckdbClient onto WorkerBus v2, delete v1 worker

Second of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication
(ADR-0014). services/duckdb/duckdbClient.ts's public API (init/query/
exec/shutdown/terminate/setOpfsFallbackHandler) is unchanged β€” its 5
real consumers (useDuckDb.ts, duckdbAnalytics.ts, duckdbMigration.ts,
ragVectorMigration.ts, telemetryService.ts) need no changes. Only the
internals swapped from a raw `new Worker(duckdbWorker.ts)` to routing
through the shared WorkerBus v2 'duckdb' pool via a new
ensureDuckDbPool() (mirrors ensureWebLlmPool()'s decoupling from the
enableWorkerBusV2 flag β€” analytics is core, not experimental infra).

The v2 worker returns raw values and throws on failure, unlike v1's
{ok,rows,error} wrapper β€” the adapter in duckdbClient.ts's send()
translates at that one boundary instead of touching ~20 call sites
across 5 files. WorkerBus's default 2-retry is disabled per-call
(retryPolicy: {maxRetries:0}) to avoid stacking under
duckdbAnalytics.ts's existing withDuckDbRetry() app-level retry.

Two things surfaced while wiring this up, both fixed here:
- WorkerBus.shutdown() tears down all 4 pools with no way to scope to
  just 'duckdb' β€” added WorkerBus.terminatePool(poolId) so
  duckdbClient.terminate() doesn't kill in-flight inference/webllm/
  plugin work on the shared bus.
- send() is async and awaits ensureDuckDbPool() before a task handle
  exists; naively attaching the AbortSignal listener after that await
  (like a first draft did) can lose a same-tick abort, unlike v1's
  synchronous Promise-executor version. Fixed by attaching the
  listener synchronously up front and applying it once the handle is
  ready β€” regression-tested.

workers/duckdbWorker.ts deleted; features/featureCatalog.ts's
enableDuckDbAnalytics entry retargeted to workers/v2/duckdb.worker.ts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): make OPFS-connection cleanup best-effort on ATTACH failure

CodeRabbit's second pass on #286 flagged this as an outside-diff-range
comment (not a resolvable inline thread) on bcc41c2's own fix: the
new `await opfsConnection?.close()` call could itself reject, which
would skip emitProgress('opfs-fallback', ...) and the fallback
newDb.connect(), and replace the original ATTACH error with the close
failure β€” turning a graceful degrade into a hard init failure.

Wrapped the cleanup in its own try/catch (best-effort, logged via
console.warn, never blocks the fallback path) so a failing close()
can't prevent the fallback connection or swallow the real error. New
regression test asserts both still happen when close() rejects.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(ai): migrate localEmbeddingService onto WorkerBus v2

Third of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication
(ADR-0014). services/ai/localEmbeddingService.ts's public API
(embedText/embedBatch/cosineSimilarity/clearEmbeddingCache) is
unchanged β€” its consumers (localRagService.ts, crossProjectIndexService.ts,
ragPromptAssembly.ts, ragVectorMigration.ts, proForgeMemoryBank.ts,
loraEvaluationService.ts, loraDatasetBuilder.ts, inferenceGateway.ts)
need no changes. Internals swapped from a dedicated
`new Worker(inference.worker.ts)` instance to the shared WorkerBus v2
'inference' pool via ensureInferencePool() (same decoupling-from-
enableWorkerBusV2 pattern as ensureWebLlmPool/ensureDuckDbPool).

v2's handler already throws on failure (matching embedText's existing
throw-on-failure contract), so no response-shape translation is
needed here β€” just swap the transport. Deleted ~70 lines of hand-rolled
health-check/restart/backoff logic (30s ping, exponential backoff up
to 5 attempts), superseded by WorkerPool's generic health check.

Capped the shared 'inference' pool's maxWorkers to 2 (was
MAX_WORKERS_INFERENCE=4) in workerBusManager.ts: each pool replica
independently loads its own transformers.js pipeline with no
cross-replica cache sharing, so 4 concurrent workers under a burst
(e.g. embedBatch's micro-batching) could mean 4x the model memory
footprint.

workers/inference.worker.ts (v1) is NOT deleted yet β€” localNlpService.ts
still uses it directly; removal is PR 4 of this sequence, after that
migration lands too.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): pool-termination lifecycle, idle-respawn recovery, SQL param binding

Three real findings from CodeRabbit's review of #287 (duckdbClient
migration onto WorkerBus v2), all confirmed against current code
before fixing:

1. CRITICAL β€” params silently dropped (workers/v2/duckdb.worker.ts).
   duckdbClient.send() enqueues {sql, params}, but handleQuery/
   handleExec only read req.sql and called connection.query(sql)
   directly, ignoring params entirely and leaving an unbound-query/
   SQL-injection surface flagged by static analysis. This was already
   live: services/ai/telemetryService.ts's writeToDuckDb() passes real
   bound params ([taskType, backend, modelId, latencyMs, success])
   that were being silently discarded. Fixed via DuckDB-WASM prepared
   statements (connection.prepare(sql) + stmt.query(...params) +
   stmt.close()) when params is non-empty; unchanged direct
   connection.query(sql) otherwise. Needed a narrow structural type
   (PreparableConnection) since tsgo doesn't resolve
   AsyncDuckDBConnection.prepare() through this package's nested
   `export *` chain, despite it being present in the package's own
   .d.ts β€” same class of tsgo/external-package gap CLAUDE.md already
   documents for the transformers.js path alias.

2. MAJOR β€” DuckDB pool worker losing its connection on respawn
   (services/duckdb/duckdbClient.ts). The duckdb pool's minWorkers:0
   lets its worker idle-terminate; a freshly spawned replacement
   worker's module state has no `connection` until INIT runs again on
   it, so the next QUERY/EXEC after a respawn failed with "DuckDB not
   initialized". Fixed by transparently re-sending INIT once and
   retrying the original call when send() sees that specific error
   message (bounded to one retry, INIT/SHUTDOWN excluded from the
   check to avoid any recursive loop).

3. MAJOR β€” terminatePool() had no lifecycle contract
   (packages/worker-bus/src/workerBus.ts +
   services/workerBusManager.ts). Removing a pool didn't settle tasks
   already routed to it β€” their result promises would await a RESULT
   message from a worker that no longer exists. Fixed by tracking
   task->pool assignment (new taskPools map, set/cleared around
   runTask's pool-acquire phase) and cancelling matching tasks before
   deletion. Separately, ensureDuckDbPool() assumed a non-null bus
   always has every pool, so any call after duckdbClient.terminate()
   would fail with NO_POOL forever; added WorkerBus.hasPool() and a
   re-registration path (duckDbPoolOptions()/reRegisterDuckDbPool(),
   shared with doInitWorkerBus() to avoid option drift between the two
   registration sites).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(ai): add the missing ensureInferencePool export that broke prod builds

Root cause of both Vercel deployment failures on this branch/PR chain:
services/ai/localEmbeddingService.ts imports and calls
ensureInferencePool() from workerBusManager.ts, but that function was
never actually added there β€” only ensureDuckDbPool() was. Vitest never
caught this because localEmbeddingService.test.ts mocks the entire
workerBusManager module (vi.mock(...) replaces it wholesale), so the
real file's missing export was invisible to that suite. tsgo's
typecheck also passed clean locally for reasons still unclear (worth
a follow-up look), but rolldown's stricter static bundling in the
actual `pnpm run build:edge` (Vercel's exact build command) correctly
failed hard with [MISSING_EXPORT] β€” reproduced locally with
`NODE_OPTIONS=--max-old-space-size=3072 pnpm run build:edge`, which is
what actually surfaced this.

Added ensureInferencePool(), mirroring ensureDuckDbPool()'s shape
(decoupled from enableWorkerBusV2, re-registers the pool if it was
removed via terminatePool() while the bus stays alive). Refactored
doInitWorkerBus()'s inline 'inference' pool registration to share the
same inferencePoolOptions() the new function uses, avoiding the same
kind of drift the duckdb pool options already guard against.

New tests in workerBusManager.test.ts import the *real* module (only
@domain/worker-bus is mocked) β€” this exact suite would have caught the
missing export immediately, unlike the fully-mocked consumer-side
test. Verified fixed via a full local `pnpm run build:edge`
reproduction of Vercel's exact build command (previously failed with
[MISSING_EXPORT], now builds clean).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(ai): migrate localNlpService onto WorkerBus v2, delete v1 inference worker

Fourth and final migration PR of the worker-generation consolidation
(ADR-0014). services/ai/localNlpService.ts's public API
(analyzeSentiment/summarizeText/classifyWritingTopic) is unchanged β€”
confirmed zero real production callers beyond its own tests and a
barrel re-export in services/ai/index.ts, so this is the
lowest-blast-radius of the three migrations. Internals swapped from a
dedicated `new Worker(inference.worker.ts)` instance to the shared
WorkerBus v2 'inference' pool via ensureInferencePool() (added in the
previous PR), routing to the 'inference.text' capability.

analyzeSentiment/summarizeText already had their own app-level
graceful-degrade behavior on failure (NEUTRAL sentiment / truncated
text) β€” that's preserved unchanged via try/catch around the new
throw-on-failure requestInference() call, replacing the old
ok:false-check.

With both localEmbeddingService.ts (previous PR) and this migrated,
workers/inference.worker.ts (v1) has no remaining callers β€” deleted,
along with its dedicated test (tests/unit/inferenceWorker.test.ts).
tests/unit/localNlpService.test.ts rewritten against a
workerBusManager mock; tests/unit/services/localNlpService.test.ts
deleted as a near-total duplicate of the rewritten suite (same module,
overlapping classifyWritingTopic/analyzeSentiment coverage, the
comprehensive file already had everything it did).

Verified via typecheck and a full local `pnpm run build:edge`
reproduction of Vercel's exact build command (the lesson from the
previous PR's missing-export incident) β€” both clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(ai): omit inferenceOptions key instead of passing undefined (exactOptionalPropertyTypes)

localNlpService.requestInference() spread inferenceOptions into the enqueue
payload via object-literal shorthand, so calls without a 4th arg (analyzeSentiment)
set the key to an explicit `undefined`. With exactOptionalPropertyTypes the
optional payload field only accepts the property being absent, not present-with-undefined.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* test(ai): close Codecov patch-coverage gap on PR #290

Codecov flagged 3 partial branches in localNlpService.ts (83.33% patch
coverage). Two were genuinely reachable and now have tests:
- ensureInferencePool() returning null (WorkerBus unavailable)
- the worker response omitting ":score" (scoreRaw undefined, ?? '0.5' fallback)

The third β€” labelRaw's `?? 'NEUTRAL'` fallback β€” was dead code:
String.split(':') never returns an empty array, so labelRaw (index 0) can
never actually be undefined; only TypeScript's noUncheckedIndexedAccess
couldn't see that. Replaced with a targeted `as string` assertion (compile-time
only, no runtime branch) instead of writing a contrived test for an
unreachable path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(test): require an enqueue call before asserting payload shape

Addresses a CodeRabbit finding on PR #290: the exactOptionalPropertyTypes
test used `requestCalls[0] ?? {}`, so it would still pass on an empty
{} object if analyzeSentiment failed to enqueue at all β€” a regression in
the enqueue path wouldn't be caught. Assert the call actually happened first.

The other finding on this review (missing QNBS-v3 comment on the two new
fallback tests) doesn't hold: none of the 8 tests in this describe block
have individual QNBS-v3 comments, only the file-level one β€” adding it to
just the 2 new tests would be less consistent, not more.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
qnbs added a commit that referenced this pull request Jul 30, 2026
…un log (#291)

* test(worker-bus): export v2 duckdb/inference handlers + parity gate tests

Neither workers/v2/duckdb.worker.ts's handlers nor
workers/v2/inference.worker.ts's handleInference were exported or
unit-tested against real logic (only webllm.worker.ts had that,
tests/unit/webllmWorkerHandler.test.ts) despite both already being
registered as live WorkerBus v2 pools with zero production callers β€”
docs/adr/0014-worker-generation-duplication.md's deferred migration
needs these to actually match v1 before any consumer cuts over.

Testing surfaced a real gap: initDuckDb()'s OPFS-unavailable catch
block silently swallowed the failure, unlike v1's out-of-band
OPFS_FALLBACK message that lets the UI warn users their analytics
won't persist. Fixed by threading ctx.emitProgress into initDuckDb()
and emitting an 'opfs-fallback' progress stage from the catch block β€”
reuses the existing progress channel WebLLM already uses for download
progress, no new message-protocol/schema change needed.

First PR of the ADR-0014 consolidation sprint (5 PRs total, see
.claude/plans/worker-generation-v1-vs-enumerated-fox.md).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): close leaked OPFS connection on ATTACH failure + QNBS-v3 format

CodeRabbit review on #286 found a real bug: initDuckDb()'s OPFS catch
block overwrote `connection` with the fallback connection without
closing the first (partially-attached) one, leaking a DuckDB
connection on every OPFS-attach failure. Fixed by tracking the OPFS
attempt in its own local variable and closing it in the catch path
before falling back. New regression test asserts close() is called.

Also collapsed several multi-line QNBS-v3 comments in
workers/v2/duckdb.worker.ts, workers/v2/inference.worker.ts, and both
new test files to the required single bracketed-line format.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(duckdb): migrate duckdbClient onto WorkerBus v2, delete v1 worker

Second of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication
(ADR-0014). services/duckdb/duckdbClient.ts's public API (init/query/
exec/shutdown/terminate/setOpfsFallbackHandler) is unchanged β€” its 5
real consumers (useDuckDb.ts, duckdbAnalytics.ts, duckdbMigration.ts,
ragVectorMigration.ts, telemetryService.ts) need no changes. Only the
internals swapped from a raw `new Worker(duckdbWorker.ts)` to routing
through the shared WorkerBus v2 'duckdb' pool via a new
ensureDuckDbPool() (mirrors ensureWebLlmPool()'s decoupling from the
enableWorkerBusV2 flag β€” analytics is core, not experimental infra).

The v2 worker returns raw values and throws on failure, unlike v1's
{ok,rows,error} wrapper β€” the adapter in duckdbClient.ts's send()
translates at that one boundary instead of touching ~20 call sites
across 5 files. WorkerBus's default 2-retry is disabled per-call
(retryPolicy: {maxRetries:0}) to avoid stacking under
duckdbAnalytics.ts's existing withDuckDbRetry() app-level retry.

Two things surfaced while wiring this up, both fixed here:
- WorkerBus.shutdown() tears down all 4 pools with no way to scope to
  just 'duckdb' β€” added WorkerBus.terminatePool(poolId) so
  duckdbClient.terminate() doesn't kill in-flight inference/webllm/
  plugin work on the shared bus.
- send() is async and awaits ensureDuckDbPool() before a task handle
  exists; naively attaching the AbortSignal listener after that await
  (like a first draft did) can lose a same-tick abort, unlike v1's
  synchronous Promise-executor version. Fixed by attaching the
  listener synchronously up front and applying it once the handle is
  ready β€” regression-tested.

workers/duckdbWorker.ts deleted; features/featureCatalog.ts's
enableDuckDbAnalytics entry retargeted to workers/v2/duckdb.worker.ts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): make OPFS-connection cleanup best-effort on ATTACH failure

CodeRabbit's second pass on #286 flagged this as an outside-diff-range
comment (not a resolvable inline thread) on bcc41c2's own fix: the
new `await opfsConnection?.close()` call could itself reject, which
would skip emitProgress('opfs-fallback', ...) and the fallback
newDb.connect(), and replace the original ATTACH error with the close
failure β€” turning a graceful degrade into a hard init failure.

Wrapped the cleanup in its own try/catch (best-effort, logged via
console.warn, never blocks the fallback path) so a failing close()
can't prevent the fallback connection or swallow the real error. New
regression test asserts both still happen when close() rejects.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(ai): migrate localEmbeddingService onto WorkerBus v2

Third of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication
(ADR-0014). services/ai/localEmbeddingService.ts's public API
(embedText/embedBatch/cosineSimilarity/clearEmbeddingCache) is
unchanged β€” its consumers (localRagService.ts, crossProjectIndexService.ts,
ragPromptAssembly.ts, ragVectorMigration.ts, proForgeMemoryBank.ts,
loraEvaluationService.ts, loraDatasetBuilder.ts, inferenceGateway.ts)
need no changes. Internals swapped from a dedicated
`new Worker(inference.worker.ts)` instance to the shared WorkerBus v2
'inference' pool via ensureInferencePool() (same decoupling-from-
enableWorkerBusV2 pattern as ensureWebLlmPool/ensureDuckDbPool).

v2's handler already throws on failure (matching embedText's existing
throw-on-failure contract), so no response-shape translation is
needed here β€” just swap the transport. Deleted ~70 lines of hand-rolled
health-check/restart/backoff logic (30s ping, exponential backoff up
to 5 attempts), superseded by WorkerPool's generic health check.

Capped the shared 'inference' pool's maxWorkers to 2 (was
MAX_WORKERS_INFERENCE=4) in workerBusManager.ts: each pool replica
independently loads its own transformers.js pipeline with no
cross-replica cache sharing, so 4 concurrent workers under a burst
(e.g. embedBatch's micro-batching) could mean 4x the model memory
footprint.

workers/inference.worker.ts (v1) is NOT deleted yet β€” localNlpService.ts
still uses it directly; removal is PR 4 of this sequence, after that
migration lands too.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worker-bus): pool-termination lifecycle, idle-respawn recovery, SQL param binding

Three real findings from CodeRabbit's review of #287 (duckdbClient
migration onto WorkerBus v2), all confirmed against current code
before fixing:

1. CRITICAL β€” params silently dropped (workers/v2/duckdb.worker.ts).
   duckdbClient.send() enqueues {sql, params}, but handleQuery/
   handleExec only read req.sql and called connection.query(sql)
   directly, ignoring params entirely and leaving an unbound-query/
   SQL-injection surface flagged by static analysis. This was already
   live: services/ai/telemetryService.ts's writeToDuckDb() passes real
   bound params ([taskType, backend, modelId, latencyMs, success])
   that were being silently discarded. Fixed via DuckDB-WASM prepared
   statements (connection.prepare(sql) + stmt.query(...params) +
   stmt.close()) when params is non-empty; unchanged direct
   connection.query(sql) otherwise. Needed a narrow structural type
   (PreparableConnection) since tsgo doesn't resolve
   AsyncDuckDBConnection.prepare() through this package's nested
   `export *` chain, despite it being present in the package's own
   .d.ts β€” same class of tsgo/external-package gap CLAUDE.md already
   documents for the transformers.js path alias.

2. MAJOR β€” DuckDB pool worker losing its connection on respawn
   (services/duckdb/duckdbClient.ts). The duckdb pool's minWorkers:0
   lets its worker idle-terminate; a freshly spawned replacement
   worker's module state has no `connection` until INIT runs again on
   it, so the next QUERY/EXEC after a respawn failed with "DuckDB not
   initialized". Fixed by transparently re-sending INIT once and
   retrying the original call when send() sees that specific error
   message (bounded to one retry, INIT/SHUTDOWN excluded from the
   check to avoid any recursive loop).

3. MAJOR β€” terminatePool() had no lifecycle contract
   (packages/worker-bus/src/workerBus.ts +
   services/workerBusManager.ts). Removing a pool didn't settle tasks
   already routed to it β€” their result promises would await a RESULT
   message from a worker that no longer exists. Fixed by tracking
   task->pool assignment (new taskPools map, set/cleared around
   runTask's pool-acquire phase) and cancelling matching tasks before
   deletion. Separately, ensureDuckDbPool() assumed a non-null bus
   always has every pool, so any call after duckdbClient.terminate()
   would fail with NO_POOL forever; added WorkerBus.hasPool() and a
   re-registration path (duckDbPoolOptions()/reRegisterDuckDbPool(),
   shared with doInitWorkerBus() to avoid option drift between the two
   registration sites).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(ai): add the missing ensureInferencePool export that broke prod builds

Root cause of both Vercel deployment failures on this branch/PR chain:
services/ai/localEmbeddingService.ts imports and calls
ensureInferencePool() from workerBusManager.ts, but that function was
never actually added there β€” only ensureDuckDbPool() was. Vitest never
caught this because localEmbeddingService.test.ts mocks the entire
workerBusManager module (vi.mock(...) replaces it wholesale), so the
real file's missing export was invisible to that suite. tsgo's
typecheck also passed clean locally for reasons still unclear (worth
a follow-up look), but rolldown's stricter static bundling in the
actual `pnpm run build:edge` (Vercel's exact build command) correctly
failed hard with [MISSING_EXPORT] β€” reproduced locally with
`NODE_OPTIONS=--max-old-space-size=3072 pnpm run build:edge`, which is
what actually surfaced this.

Added ensureInferencePool(), mirroring ensureDuckDbPool()'s shape
(decoupled from enableWorkerBusV2, re-registers the pool if it was
removed via terminatePool() while the bus stays alive). Refactored
doInitWorkerBus()'s inline 'inference' pool registration to share the
same inferencePoolOptions() the new function uses, avoiding the same
kind of drift the duckdb pool options already guard against.

New tests in workerBusManager.test.ts import the *real* module (only
@domain/worker-bus is mocked) β€” this exact suite would have caught the
missing export immediately, unlike the fully-mocked consumer-side
test. Verified fixed via a full local `pnpm run build:edge`
reproduction of Vercel's exact build command (previously failed with
[MISSING_EXPORT], now builds clean).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* feat(ai): migrate localNlpService onto WorkerBus v2, delete v1 inference worker

Fourth and final migration PR of the worker-generation consolidation
(ADR-0014). services/ai/localNlpService.ts's public API
(analyzeSentiment/summarizeText/classifyWritingTopic) is unchanged β€”
confirmed zero real production callers beyond its own tests and a
barrel re-export in services/ai/index.ts, so this is the
lowest-blast-radius of the three migrations. Internals swapped from a
dedicated `new Worker(inference.worker.ts)` instance to the shared
WorkerBus v2 'inference' pool via ensureInferencePool() (added in the
previous PR), routing to the 'inference.text' capability.

analyzeSentiment/summarizeText already had their own app-level
graceful-degrade behavior on failure (NEUTRAL sentiment / truncated
text) β€” that's preserved unchanged via try/catch around the new
throw-on-failure requestInference() call, replacing the old
ok:false-check.

With both localEmbeddingService.ts (previous PR) and this migrated,
workers/inference.worker.ts (v1) has no remaining callers β€” deleted,
along with its dedicated test (tests/unit/inferenceWorker.test.ts).
tests/unit/localNlpService.test.ts rewritten against a
workerBusManager mock; tests/unit/services/localNlpService.test.ts
deleted as a near-total duplicate of the rewritten suite (same module,
overlapping classifyWritingTopic/analyzeSentiment coverage, the
comprehensive file already had everything it did).

Verified via typecheck and a full local `pnpm run build:edge`
reproduction of Vercel's exact build command (the lesson from the
previous PR's missing-export incident) β€” both clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs: ADR-0015, CLAUDE.md/AGENTS.md doc-truth, TODO/AUDIT closeout, run log

Closes out the worker-generation consolidation sprint (PRs #286-288,
#290) at the documentation layer.

- New docs/adr/0015-worker-generation-consolidation.md β€” the decision
  record for the completed migration (target generation, per-workload
  changes, parity gaps found and fixed, deliberately-unchanged
  WebLLM-fallback-shape rationale, the timeoutMs follow-up).
- docs/adr/0014-worker-generation-duplication.md: status line changed
  to "Superseded by 0015" (body untouched, per this repo's own
  "supersede, don't edit history" convention).
- docs/adr/README.md: index gained rows 0010-0015 β€” 0010-0014 were
  pre-existing files missing from the index before this sprint,
  unrelated drift fixed opportunistically while editing the file.
- CLAUDE.md/AGENTS.md: directory map, WorkerBus v2 section, DuckDB
  section, and Known Technical Debt all updated to the post-migration
  state. Also fixed 2 pre-existing doc-truth bugs found while reading
  the WorkerBus v2 source directly: WorkerHandlerContext.emitProgress
  is a flat function, not context.progress.emit(...); there is no
  'background' priority tier, only critical > high > normal > low.
- TODO.md: F-14 entry marked done with the PR chain and correction-
  loop findings. Also caught and fixed a genuinely stale, unrelated
  entry while in the file β€” "Tag v1.24.2 + publish release" was still
  marked open, but v1.24.2 was already tagged/released (verified via
  git tag/gh release list).
- AUDIT.md: F-14 and F-11 (the same stale-tag issue) findings updated
  to resolved.
- CHANGELOG.md: [Unreleased] section added, covering the full sprint
  (Changed/Fixed/Removed).
- New docs/audit/WS-RUN-LOG-2026-07-29-worker-consolidation.md β€”
  execution log in this repo's established WS-RUN-LOG format: baseline,
  the 4-PR breakdown, every correction-loop finding with its fix, the
  Vercel missing-export incident, final verification, and what's
  deliberately left unresolved (timeoutMs enforcement; why local tsgo
  typecheck didn't catch the missing export).

Verified: pnpm exec tsx scripts/audit-feature-parity.ts (0 drifts),
checkDocMetrics.test.ts + featureCatalog.test.ts (28/28), typecheck
clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(docs): don't recommend @ts-expect-error as a suppression fallback

CodeRabbit review on #291 correctly flagged that the Known Technical
Debt line I edited (only updating the file path to
workers/v2/inference.worker.ts) still carried a pre-existing
recommendation to "restore @ts-expect-error" if the transformers.js
path alias breaks β€” in direct tension with this repo's own
suppression-ratchet policy documented a few sections above in the
same file. Reworded to point at fixing the alias/type declaration
directly instead, consistent with the structural-type workaround this
same sprint used for a different tsgo/external-package resolution gap
(workers/v2/duckdb.worker.ts's PreparableConnection, PR #287) rather
than reaching for a suppression.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@qnbs qnbs mentioned this pull request Jul 30, 2026
4 tasks
qnbs added a commit that referenced this pull request Jul 30, 2026
package.json bumped; src-tauri/tauri.conf.json, src-tauri/Cargo.toml,
src-tauri/Cargo.lock (workspace-member entry, no checksum to break), and
public/sw.js's APP_VERSION synced via the existing sync-tauri-version.mjs /
sync-sw-version.mjs scripts plus one direct lockfile edit. README version
badge updated.

New CHANGELOG.md [1.24.3] entry finalizes the Unreleased worker-generation
consolidation section: corrects the stacked-PR count (4 -> 5, #286-288 +
#290-291), folds in independent #289 (Vercel auto-deploy pause), and adds
two additional fixes from this session's merge-queue recovery (abort-listener
leak in duckdbClient.ts, pool re-registration failure guards in
workerBusManager.ts) that weren't yet documented.

Tag creation and GitHub Release publishing follow in a separate step, per
explicit maintainer authorization for this recovery.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant